Merged
Conversation
Contributor
Author
|
@claude[agent] review |
I've completed a comprehensive code review of this PR. Here's my assessment: Summary✅ Code quality is excellent - All tests pass, no issues found. Test Results
Review FindingsWhat's Working Well
Changes Reviewed
Security & Performance
The implementation is ready for merge. Commits: 4f1c746, f3a0fbc, 01fab74, be019ac, 47ec188 |
9f59c91 to
134e099
Compare
oliviergaraud
approved these changes
Apr 13, 2026
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #262 +/- ##
==========================================
+ Coverage 89.03% 89.18% +0.15%
==========================================
Files 75 77 +2
Lines 10060 10265 +205
==========================================
+ Hits 8957 9155 +198
- Misses 1103 1110 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The pipes module was removed in Python 3.13. shlex.quote is the recommended replacement and has been available since Python 3.3. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Set merge.ff=true in the local clone so that git fast-forwards when possible, regardless of any merge.ff=no set in the operator's global gitconfig. The queueing mechanism relies on q/<version> being fast-forwarded to pr.src_commit so that handle_commit can locate the queue branch from the commit sha. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Before GA (no 10.0.0.X tag exists), a hotfix/X.Y.Z branch may already exist to receive the first GA patch series. Previously bert-e only handled hotfix branches post-GA (once a tag existed). This extends three areas: - HotfixBranch.hfrev now defaults to 0 instead of -1, and version is initialised to X.Y.Z.0. update_versions already increments hfrev past any existing tag, so post-GA behaviour is preserved. - BranchCascade stores hotfix branches as phantom entries when the PR destination is a development branch. This allows _update_major_versions to see minor=0 from hotfix/10.0.0 and produce target 10.1.0 for development/10, mirroring the post-GA behaviour that previously required the 10.0.0.0 tag to be present. - get_queue_integration_branch uses hfrev >= 0 (was > 0) to recognise pre-GA hotfix branches when naming the queue integration branch, producing q/w/PR/10.0.0.0/... instead of falling back to the integration branch version. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add hotfix/10.0.0 to the mock test repository setup so integration tests can target it. - test_branch_cascade_target_hotfix: add a pre-GA case (no 6.6.6.X tag) to verify the target version is 6.6.6.0. - test_branch_cascade_dev_with_pre_ga_hotfix: verify that when hotfix/10.0.0 exists but no 10.0.0.X tag is present, development/10 targets 10.1.0 (same result as post-GA). - test_pr_hotfix_pre_ga: end-to-end integration test — PR targeting hotfix/10.0.0 pre-GA is queued under q/w/PR/10.0.0.0/..., build succeeds, and the PR merges cleanly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Verifies the full coexistence of development/9.5.3, hotfix/10.0.0 (pre-GA), development/10.0.1, development/10.1, and development/10: - Cascade from dev/9.5.3 goes through 10.0.1 -> 10.1 -> 10 - hotfix/10.0.0 (phantom) does not appear in merge paths or ignored list - dev/10.latest_minor is driven by dev/10.1 (minor=1), so dev/10 targets 10.2.0 regardless of the phantom hotfix contributing minor=0 - 3-digit branches target their own version when no 2-digit ancestor is present in the cascade - Post-GA (10.0.0.0 tag present): same targets, tag is effectively ignored since dev/10.0 and dev/10.0.0 are absent from the cascade Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…0.0.0 Verifies dev/9.5, hotfix/10.0.0, dev/10.0, dev/10: - Pre-GA: dev/10.0 targets 10.0.0, dev/10 targets 10.1.0 - Post-GA (10.0.0.0 tag): dev/10.0 advances to 10.0.1, dev/10 unchanged Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two regressions introduced when HotfixBranch.version was changed to include .0 suffix by default (hfrev=0 class attribute): 1. create_branch.py: archive-tag check fired for HotfixBranch because version='X.Y.Z.0' matched the GA release tag 'X.Y.Z.0'. Skip this check for hotfix branches since their archive tags use the format 'X.Y.Z.hfrev.archived_hotfix_branch'. Also fix branch_from to use the 3-digit base form '%d.%d.%d.0' instead of version+'.0' which would produce 'X.Y.Z.0.0'. 2. branches.py: has_version_queued_prs() was only prefix-matching for hfrev==-1 (legacy sentinel), missing the case where branch_factory() creates a HotfixBranch with hfrev==0 but update_versions() already advanced the queue to hfrev==1 (post-GA). Extend the fallback prefix match to cover hfrev==0 as well, with an exact-match-first strategy to preserve correct pre-GA behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The condition `hfrev >= 0` in get_queue_integration_branch() was intended to mean "this is a hotfix destination", relying on the accident that DevelopmentBranch inherits hfrev=-1 from GWFBranch. Any future branch type with hfrev=0 would silently trigger the wrong code path. Replace with an explicit isinstance(dst, HotfixBranch) check, which expresses the intent directly and is robust to future branch types. Add a unit test that patches hfrev=0 onto a DevelopmentBranch and verifies the queue name still uses wbranch.version (not dst.version). The test fails on the old `hfrev >= 0` guard. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Phantom hotfixes (stored in _phantom_hotfixes for non-hotfix PR cascades) were never iterated by update_versions(), leaving their .hfrev and .version frozen at the initial pre-GA values forever. Today only .minor is consumed (_update_major_versions), so there is no live bug, but any future caller reading .hfrev or .version would silently get stale data. Apply the same hfrev advancement logic to phantom hotfixes inside update_versions() so their state stays consistent with the cascade. Add a QuickTest that builds a cascade with a phantom hotfix, processes a GA tag, and asserts hfrev advances to 1. The test fails without the fix because the phantom is never updated. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…A tag
After a GA tag is pushed (e.g. 10.0.0.0), HotfixBranch.hfrev advances from
0 to 1. The next PR handle call would compute queue name q/w/{id}/10.0.0.1/...
which does not exist, causing already_in_queue to return False and re-queue
the PR — creating an orphaned q/10.0.0.1 branch alongside the valid one.
Fix: after the exact-match lookup, fall back to a git branch -r --list prefix
scan (origin/q/w/{id}/major.minor.micro.*) so the existing pre-GA queue branch
is found regardless of which hfrev was in effect when the PR was first queued.
Add test_pr_hotfix_no_requeue_after_ga: queues pre-GA, pushes 10.0.0.0 tag,
then re-handles the PR — asserts NothingToDo (not Queued) and verifies no
orphaned 10.0.0.1 branch is created.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1c8b3ce to
3f1c9a1
Compare
matthiasL-scality
approved these changes
Apr 14, 2026
rdebay-scality
approved these changes
Apr 14, 2026
When hotfix/X.Y.Z has no GA tag yet (hfrev == 0, target version ends
in ".0"), the Jira project may not have an "X.Y.Z.0" release entry yet.
Allow either the 4-digit form ("10.0.0.0") OR the 3-digit base ("10.0.0")
to satisfy the fix-version check.
Once the GA tag is pushed, hfrev advances to 1, the target becomes
"10.0.0.1", and only that exact 4-digit version is accepted again —
matching the existing post-GA behaviour.
Also update the incorrect_fix_version.md template: the condition now
checks whether the last expected version has 4 digits (covers both
pre-GA with two alternatives and post-GA with one), replacing the
prior "length == 1" check that would miss the pre-GA two-version case.
Add bert_e/tests/unit/test_jira.py with 13 unit tests covering:
- pre-GA: 4-digit accepted, 3-digit accepted, wrong/missing rejected
- post-GA: correct hfrev accepted, 3-digit base and wrong hfrev rejected
- dev-branch: existing behaviour unchanged
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…m dev check When hotfix/X.Y.Z is created pre-GA (hfrev=0, version X.Y.Z.0), and a PR targets a development branch in the same cascade, two things must happen: 1. **Jira fix-version check for the dev-branch PR** The ticket now carries three versions: e.g. 9.5.3, 10.0.0.0, 10.1.0. The 10.0.0.0 entry belongs to the hotfix branch and will be consumed by a separate cherry-pick PR; it must not cause the dev-branch check to fail. BranchCascade.phantom_hotfix_versions returns the X.Y.Z.0 form for each pre-GA phantom hotfix, and check_fix_versions subtracts them from checked_versions before comparing against expected_versions. Also drop the 3-digit-acceptance added in the previous commit: hotfix branches always require an exact 4-digit fix version (10.0.0.0 pre-GA, 10.0.0.1 post-GA first hotfix, etc.) — 3-digit aliases are rejected. 2. **Reminder message in Queued / SuccessMessage** BranchCascade.pending_hotfix_branches exposes phantom hotfixes to the templates. queued.md and successful_merge.md now include a warning block listing branches that need a separate cherry-pick PR. Queued.__init__ accepts pending_hotfixes= (default []) for backward compatibility; all raise sites pass job.git.cascade.pending_hotfix_branches. Tests added: - test_jira.py: 14 unit tests covering pre-GA/post-GA hotfix acceptance, 3-digit rejection, phantom exclusion for dev PRs, and no-phantom rejection. - test_bert_e.py: test_dev_pr_reminder_about_pre_ga_hotfix verifies the Queued message mentions hotfix/10.0.0 for a development/10 PR. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When a dev-branch PR's Jira ticket contains a X.Y.Z.0 fix version (belonging to a pre-GA hotfix branch), bert-e now posts an informational comment reminding the developer to open a cherry-pick PR to the corresponding hotfix branch. - New PendingHotfixVersionReminder exception (code 223, InformationException) - New template pending_hotfix_version_reminder.md - _notify_pending_hotfix_if_needed() called after check_fix_versions() in jira_checks; posts at most once per PR (NEVER_REPEAT) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
handle_legacy() re-raises SuccessMessage from a Queued exception's saved fields, but omitted the pending_hotfixes field added in 0d0cdca. With StrictUndefined in the Jinja2 environment this caused UndefinedError in successful_merge.md for every TestBertE test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements and fixes to the versioning and cascade logic for hotfix and development branches, especially in pre-GA (General Availability) scenarios. The changes ensure that hotfix branches without a GA tag are handled correctly, development branches target the right versions when hotfixes exist, and the test suite covers these edge cases. Additionally, minor improvements to git configuration and quoting logic are included.
Hotfix and Development Branch Cascade Logic Improvements:
x.y.z.0before a GA tag exists, and this is reflected in the newhfrev = 0default and initialization logic inHotfixBranch, ensuring pre-GA hotfixes are versioned correctly.development/10) now correctly advance their target version (e.g., to10.1.0) when a pre-GA hotfix branch (e.g.,hotfix/10.0.0) exists, even if no GA tag is present. This is achieved by tracking "phantom" hotfixes for version calculation without including them in the actual cascade. [1] [2] [3] [4]Test Suite Enhancements:
Queue and Build Logic Adjustments:
hfrev >= 0(pre-GA or post-GA) to use their version for the queue branch, ensuring correct queue branch naming and Jira fix version assignment.Other Improvements:
pipes.quotetoshlex.quotefor shell quoting, improving compatibility and security.clone_git_repo, ensuring consistent merge behavior.